Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix generating lingua.selector url to other language on HTTPS #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mark-H
Copy link

@Mark-H Mark-H commented Mar 11, 2021

Line 208 of the lingua.selector snippet has the following preg_replace to generate the other language url:

$pageURL = preg_replace('/^' . preg_quote($parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $requestUri, '/') . '/i'
       , $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $baseUrl . $itemUri
       , $originPageUrl);

That's trying to find $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $requestUri in $originPageUrl, to replace it with the (correct) $parseUrl['scheme'] . '://' . $parseUrl['host'] . '/' . $baseUrl . $itemUri.

Because HTTPS is (transparently) served on port 443, the port is injected into $pageURL here in line 55 and therefore around line 175 into the subject of this replace, $originPageUrl.

Because $parseUrl['host'] does not have the port, the preg_replace fails and the user is given the current link with ?lang=foo added. While that works to switch the language, the user wants to see the proper localised url and not the current one.

By not adding the port if it's 443, the preg_replace matches as expected and the right URL is returned.

@JoshuaLuckers
Copy link
Collaborator

@Mark-H
Copy link
Author

Mark-H commented Mar 12, 2021

On line 50 of the snippet, sure. Would make it work better on reverse proxy setups too. It still needs this fix to make sure the preg_replace works correctly, though.

It might also make sense to re-evaluate this entire snippet as it's all very confusing to me. It seems that most of the complexity in the snippet stems from wanting to keep query parameters for the new URL, which I'd imagine can be done much simpler (and less prone to issues like this) by just using $_GET to create the new URL.... but as I'm not familiar with the full history of the project and only have this one client using Lingua that has asked me to look into some issues, I'm most likely overlooking something important that led to it being done this way in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants